gh-145411: Add runtime (now: unix only) control to Tachyon#149958
gh-145411: Add runtime (now: unix only) control to Tachyon#149958maurycy wants to merge 7 commits into
unix only) control to Tachyon#149958Conversation
| if conn in self._connections: | ||
| self._flush_connection(conn) | ||
|
|
||
| def _dispatch(self, conn, command): |
There was a problem hiding this comment.
No dump / snapshot yet.
| if getattr(args, 'control', None) is not None: | ||
| if args.subprocesses: | ||
| parser.error("--control is incompatible with --subprocesses.") | ||
| if os.name == "nt": |
There was a problem hiding this comment.
Unfortunately, I don't have access to Windows.
|
|
||
| if getattr(args, 'control', None) is not None: | ||
| if args.subprocesses: | ||
| parser.error("--control is incompatible with --subprocesses.") |
There was a problem hiding this comment.
@pablogsal Proper communication with children would be very complex. Maybe we can just use signals for enabling and disabling them.
| from .errors import ControlError, ControlURIError | ||
|
|
||
|
|
||
| class ProfilerControl: |
There was a problem hiding this comment.
I did not want to inject this into LiveCollector but the direction is clear.
unix only) control to Tachyon
|
|
||
| with _get_child_monitor_context(args, args.pid): | ||
| server = None | ||
| with ExitStack() as stack: |
There was a problem hiding this comment.
Keeping should-be-soon-to-be-added signal (SIGUSR1, SIGUSR2 for enabling or disabling?) support in mind.
| self.close_after_write = False | ||
|
|
||
|
|
||
| class ControlServer: |
There was a problem hiding this comment.
Thanks for the PR @maurycy, this looks really interesting. I think we should introduce an abstraction here to be able to implement other transport protocols. Most importantly, I think we will need named pipes on Windows, but it would also allow things like adding TCP transport later down the line.
I spoke with @pablogsal briefly about this and we agreed that ideally we'd have Windows support also included in this change. Do you think you could look into that? I can run Windows in a VM and may be able to help out, but I'm not sure when I'll be able to get around to it.
There was a problem hiding this comment.
+1 on the abstraction, it would be great to have everything here. I’d also love to see the snapshot command in this change.
Only
unix:for starters:Supported commands are
enable,disable,ping,statusandquit. Each command is confirmed with eitherokorerr.Example session:
Closes #149958
profiling.sampling#145411